-
-
Notifications
You must be signed in to change notification settings - Fork 6.4k
feat(remark-lint): add strict rules #8106
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #8106 +/- ##
==========================================
+ Coverage 76.61% 76.78% +0.16%
==========================================
Files 115 117 +2
Lines 9610 9696 +86
Branches 322 334 +12
==========================================
+ Hits 7363 7445 +82
- Misses 2246 2250 +4
Partials 1 1 ☔ View full report in Codecov by Sentry. |
@nodejs/web what other lint rules are needed? I'm thinking:
What else? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OOC, when we add new rules, in which repositories are they being applied? doc-kit? So it means Node's core API docs? Shouldn't then any PR to remark-lint actually require nodejs/collaborators as code owners?
My plan is for these to be applied to The previous implementation did not have any CODEOWNERS, thus, this is the same, however, that can change |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGMT !
Lighthouse Results
|
2f428e0
to
0c44bd1
Compare
I'm going to rebase this, as there have been no objections from core collaborators |
Co-authored-by: Ethan Arrowood <[email protected]> Signed-off-by: Aviv Keller <[email protected]>
7eb2bd9
to
1657f6e
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR adds strict linting rules by integrating @nodejs/doc-kit
functionality to ensure type reference validation and improve existing rules. The changes move from a basic linter implementation to one that leverages doc-kit's utilities for more comprehensive validation.
- Adds a new
invalid-type-reference
rule that validates type references using doc-kit's parser utilities - Updates the
duplicate-stability-nodes
rule to use doc-kit's queries instead of hardcoded regex patterns - Enhances the
hashed-self-reference
rule to automatically fix URLs
Reviewed Changes
Copilot reviewed 7 out of 8 changed files in this pull request and generated 2 comments.
Show a summary per file
File | Description |
---|---|
packages/remark-lint/src/rules/invalid-type-reference.mjs | New rule that validates type references are properly formatted and valid using doc-kit utilities |
packages/remark-lint/src/rules/hashed-self-reference.mjs | Adds automatic URL correction to fix self-references |
packages/remark-lint/src/rules/duplicate-stability-nodes.mjs | Refactors to use doc-kit queries and improves duplicate detection logic |
packages/remark-lint/src/rules/tests/invalid-type-reference.test.mjs | Test cases for the new invalid-type-reference rule |
packages/remark-lint/src/api.mjs | Registers the new invalid-type-reference rule and adds 'npm' to allowed terminology |
packages/remark-lint/package.json | Bumps version to 1.1.0 and adds doc-kit dependency |
packages/remark-lint/README.md | Documents the new invalid-type-reference rule |
Files not reviewed (1)
- pnpm-lock.yaml: Language not supported
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
Follow-up #8057.
Now that we have a basic package, this PR modifies the rules to use
@nodejs/doc-kit
's functionality, ensuring that any changes their get reflected in the linter.Why was this separate from #8057? I wanted a version (
1.0.0
) which is almost a drop-in replacement for the original linter, without relying on@nodejs/doc-kit
, in case something broke.